Create text descriptions for e2e tests - Closes #1516 #1523

Efefefef · 2018-11-27T14:04:37Z

What issue have I solved?

-- #1516

How have I implemented/fixed it?

extracted selectors to separate file
created textual descriptions for test

How has this been tested?

Review checklist

The PR follows our Test guide
The PR follows our CSS guide

osvaldovega

In general the changes that I mentioned are related the way the comment was wrote, in some cases I am not able to understand what you what to say and in others I better use a better explanation.
In the other hand is great

osvaldovega · 2018-11-29T15:31:19Z

test/cypress/e2e/activity.spec.js

+    /**
+     * 5 transaction are shown in the latest activity
+     * @expect 5 transactions
+     */


Plase try write something more easy to understand like display 5 transaction in the last activity page or component, it is ok but something easy to understand will be better

I like Andrei's description form, but agree it could say latest activity component/module

osvaldovega · 2018-11-29T15:32:40Z

test/cypress/e2e/activity.spec.js

+     * Click on transaction row leads to tx details page
+     * @expect url
+     * @expect some specific to page element is present on it
+     */


Same here, like go to details page when a row is being click in this page or component

I think both descriptions (Andrei's and Osvaldo's) are the same clear.

osvaldovega · 2018-11-29T15:34:51Z

test/cypress/e2e/activity.spec.js

+      /**
+       * Scrolling down triggers loading another portion of txs
+       * @expect more txs are present
+       */


same here like, load more transactions when scroll is placed in the bottom of the page.

same here, same amount of clearness to me.

osvaldovega · 2018-11-29T15:36:04Z

test/cypress/e2e/activity.spec.js

+         * Maximum possible number of voted accounts is shown
+         * @expect 101 are shown
+         */
+        it('Shows 101 votes', () => {


here too, like Display the maximum amount of voted delegates (101)

here too :-) Andrei's passive tense is fine.

osvaldovega · 2018-11-29T15:37:05Z

test/cypress/e2e/activity.spec.js

+        /**
+         * Shows voted delegate's nickname not addresses
+         * @expect delegate's nickname shown
+         */


Please change this too, for be honest I think I know what you meant to say but dont understand for be honest so, I dont know how should be display

This can be better English. What about Shows voted delegate's nickname, not address?

osvaldovega · 2018-11-29T15:53:36Z

test/cypress/e2e/activity.spec.js

+        /**
+         * Shows who-voted-for-this-delegate's nickname if it was delegate
+         * @expect voters nickname shown
+         */


this comment is hard to understand can you please change it

I understand what is meant by who-voted-for-this-delegate and why the dashes, but I also get that it's not clear to anyone. What about the following?
Shows nickname of account on "Who voted for this delegate?" list if the account is a delegate

osvaldovega · 2018-11-29T16:18:06Z

test/cypress/e2e/activity.spec.js

+        /**
+         * Shows who-voted-for-this-delegate's address if it was not-delegate
+         * @expect voters address shown
+         */


as this is a comment you can remove the dash, and maybe if possible put something more descriptive

osvaldovega · 2018-11-29T16:19:43Z

test/cypress/e2e/delegateReg.spec.js

+   * @expect transaction appears in the activity list in the confirmed state with valid details
+   * @expect header balance value is decreased
+   */
+  it('Register delegate + Header balance is affected', function () {


can you please mention what are the steps of the process instead of process?

I agree. The steps can be described either in the comment above or inside the it

osvaldovega · 2018-11-29T16:20:44Z

test/cypress/e2e/delegates.spec.js

+   * Search functioning correctly
+   * @expect no delegates are shown if search for non-existing name
+   * @expect delegate is shown in 1 row when is found
+   */


for this I dont understand what exactly means

@Efefefef please ask @osvaldovega what is he missing here.

osvaldovega · 2018-11-29T16:22:08Z

test/cypress/e2e/login.spec.js

+  /**
+   * ?showNetwork parameter makes network switcher available
+   * @expect network switcher is visible
+   */


I think you missed a typo **?**showNetwork

I think it's on purpose as url param

slaweet · 2018-12-03T13:23:36Z

test/cypress/e2e/activity.spec.js

+     * Click on transaction row leads to tx details page
+     * @expect url
+     * @expect some specific to page element is present on it
+     */


I think both descriptions (Andrei's and Osvaldo's) are the same clear.

slaweet · 2018-12-03T13:25:34Z

test/cypress/e2e/activity.spec.js

+    /**
+     * 5 transaction are shown in the latest activity
+     * @expect 5 transactions
+     */


I like Andrei's description form, but agree it could say latest activity component/module

slaweet · 2018-12-03T13:26:23Z

test/cypress/e2e/activity.spec.js

+      /**
+       * Scrolling down triggers loading another portion of txs
+       * @expect more txs are present
+       */


same here, same amount of clearness to me.

slaweet · 2018-12-03T13:27:11Z

test/cypress/e2e/activity.spec.js

+         * Maximum possible number of voted accounts is shown
+         * @expect 101 are shown
+         */
+        it('Shows 101 votes', () => {


here too :-) Andrei's passive tense is fine.

slaweet · 2018-12-03T13:30:50Z

test/cypress/e2e/activity.spec.js

@@ -124,44 +137,53 @@ describe('Dashboard Activity', () => {
    describe(testSet.name, () => {
      beforeEach(() => {
        cy.autologin(accounts.delegate.passphrase, networks.devnet.node);
+        testSet.open();
+        waitBeforeChangeTabAfterLoading();
+        cy.get(ss.delegateStatisticsTab).click();


The testing talk at React Day was talking about not doing this - each it should contain the setup, if it's repetitive like this, create a helper function and call it in it.

slaweet · 2018-12-03T13:32:59Z

test/cypress/e2e/activity.spec.js

+        /**
+         * Shows voted delegate's nickname not addresses
+         * @expect delegate's nickname shown
+         */


This can be better English. What about Shows voted delegate's nickname, not address?

slaweet · 2018-12-03T13:37:01Z

test/cypress/e2e/activity.spec.js

+        /**
+         * Shows who-voted-for-this-delegate's nickname if it was delegate
+         * @expect voters nickname shown
+         */


I understand what is meant by who-voted-for-this-delegate and why the dashes, but I also get that it's not clear to anyone. What about the following?
Shows nickname of account on "Who voted for this delegate?" list if the account is a delegate

slaweet · 2018-12-03T13:38:43Z

test/cypress/e2e/delegateReg.spec.js

+   * @expect transaction appears in the activity list in the confirmed state with valid details
+   * @expect header balance value is decreased
+   */
+  it('Register delegate + Header balance is affected', function () {


I agree. The steps can be described either in the comment above or inside the it

slaweet · 2018-12-03T13:40:29Z

test/cypress/e2e/delegates.spec.js

+   * Search functioning correctly
+   * @expect no delegates are shown if search for non-existing name
+   * @expect delegate is shown in 1 row when is found
+   */


@Efefefef please ask @osvaldovega what is he missing here.

slaweet · 2018-12-03T13:41:04Z

test/cypress/e2e/login.spec.js

+  /**
+   * ?showNetwork parameter makes network switcher available
+   * @expect network switcher is visible
+   */


I think it's on purpose as url param

Efefefef · 2018-12-04T10:03:28Z

@osvaldovega I use passive voice to stress on the assertion that is the essence of the test.
For example, I would prefer 'The pie is eaten' on 'Eat the pie' because it's not clear what actual result we are actually testing in the second case. The test could check if 'my throat is safe' since there were no needles inside, but the process still would be 'Eat the pie'. So the goal of the description is to show the idea of what is being checked behind this very test, having in mind it might not be particularly clear from steps done in the test and answer questions like ' can I assert some different fields? ' / 'can I reorder steps in some way ?' and keep the idea, not making the test useless

…com:LiskHQ/lisk-hub into 1516-create-text-descriptions

slaweet

Thank you, Andrei

Changes implemented

andreiklimenok added 2 commits November 22, 2018 17:20

✅ work on TODOs

6a5fb9e

update cypress version

8702531

Efefefef requested a review from slaweet November 27, 2018 14:04

Efefefef self-assigned this Nov 27, 2018

Efefefef added the 🏗️ in progress label Nov 27, 2018

✅ textual descriptions and selectors extraction

b1ec74d

Efefefef force-pushed the 1516-create-text-descriptions branch from 49a6344 to b1ec74d Compare November 27, 2018 15:26

Efefefef added ☑️ pending QA and removed 🏗️ in progress labels Nov 27, 2018

slaweet requested review from michaeltomasik and osvaldovega November 28, 2018 14:24

slaweet changed the base branch from 1.7.0 to 1.8.0 November 29, 2018 16:22

osvaldovega previously requested changes Nov 29, 2018

View reviewed changes

osvaldovega added 🏗️ in progress and removed 👁️ pending review labels Dec 1, 2018

slaweet reviewed Dec 3, 2018

View reviewed changes

andreiklimenok added 4 commits December 4, 2018 11:53

✅ some comments edit

5087823

Merge branches '1.8.0' and '1516-create-text-descriptions' of github.…

ee46342

…com:LiskHQ/lisk-hub into 1516-create-text-descriptions

✅ update package-lock.json

389ce57

✅ skipping header updates

91dbcad

Efefefef added 👁️ pending review and removed 🏗️ in progress labels Dec 4, 2018

slaweet approved these changes Dec 4, 2018

View reviewed changes

slaweet added ✅ ready and removed 👁️ pending review labels Dec 4, 2018

Efefefef merged commit dd6ba39 into 1.8.0 Dec 4, 2018

Efefefef deleted the 1516-create-text-descriptions branch December 4, 2018 15:48

slaweet changed the title ~~Create text descriptions - Closes #1516~~ Create text descriptions for e2e tests - Closes #1516 Dec 5, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create text descriptions for e2e tests - Closes #1516 #1523

Create text descriptions for e2e tests - Closes #1516 #1523

Efefefef commented Nov 27, 2018

osvaldovega left a comment

osvaldovega Nov 29, 2018

slaweet Dec 3, 2018

osvaldovega Nov 29, 2018

slaweet Dec 3, 2018

osvaldovega Nov 29, 2018

slaweet Dec 3, 2018

osvaldovega Nov 29, 2018

slaweet Dec 3, 2018

osvaldovega Nov 29, 2018

slaweet Dec 3, 2018

osvaldovega Nov 29, 2018

slaweet Dec 3, 2018

osvaldovega Nov 29, 2018

osvaldovega Nov 29, 2018

slaweet Dec 3, 2018

osvaldovega Nov 29, 2018

slaweet Dec 3, 2018

osvaldovega Nov 29, 2018

slaweet Dec 3, 2018

slaweet Dec 3, 2018

slaweet Dec 3, 2018

slaweet Dec 3, 2018

slaweet Dec 3, 2018

slaweet Dec 3, 2018

slaweet Dec 3, 2018

slaweet Dec 3, 2018

slaweet Dec 3, 2018

slaweet Dec 3, 2018

slaweet Dec 3, 2018

Efefefef commented Dec 4, 2018 •

edited

Loading

slaweet left a comment

Create text descriptions for e2e tests - Closes #1516 #1523

Create text descriptions for e2e tests - Closes #1516 #1523

Conversation

Efefefef commented Nov 27, 2018

What issue have I solved?

How have I implemented/fixed it?

How has this been tested?

Review checklist

osvaldovega left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Efefefef commented Dec 4, 2018 • edited Loading

slaweet left a comment

Choose a reason for hiding this comment

Efefefef commented Dec 4, 2018 •

edited

Loading